Skip to content

fix(degreeworks): Provide Polymorphic requirements for majors with specializations#310

Open
HwijungK wants to merge 104 commits intomainfrom
polymorphic-majors
Open

fix(degreeworks): Provide Polymorphic requirements for majors with specializations#310
HwijungK wants to merge 104 commits intomainfrom
polymorphic-majors

Conversation

@HwijungK
Copy link
Collaborator

@HwijungK HwijungK commented Feb 17, 2026

Description

We currently save one requirement for a major. However, when majors are taken with a specialization the original major block may change. For example, the Applied and Computational Math major has 2 optional specializations. The major requirement has a "Requirements for Pure Applied and Computation Math" subset, which is removed when specializations are taken.

We now run a major scrape for each major, spec pair. The major_spec_pair_to_requirement table refers to a major code and an optional potentially null spec code, and has a requirement_id that points to another table, major_requirement. To enforce uniqueness on major_requirement, a requirements_hash column is used as the json for requirements is to large to put a unique constrain on.

The Major Requirement Endpoint has been changed to take an optional specializationId (ex = 'BS-201A) along with the programId (BS-201). It is an error to give specializationId as null or to give a specializationID that is not associated with the given majorId.

Related Issue

#263

Motivation and Context

How Has This Been Tested?

Locally tested on graphql and rest endpoints.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code involves a change to the database schema.
  • My code requires a change to the documentation.

…that points to seperate major_requirement table
Copy link
Member

@laggycomputer laggycomputer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also accidentally checked two testing files in, no big deal

majorCode: string;
specCode?: string;
};
// 'majorName;specCode'. If no specialization is taken, 'majorName;undefined'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just drop the semicolon and everything after it if there's no specialization. Explicitly leaving "undefined" in a string may be mistaken for a bug.

}),
specializationId: programIdBase.optional().openapi({
description:
"fetch major requirements when taking the specialization with this ID, if provided; providing no specialization when one is required returns the 'base' major requirements",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

providing no specialization when one is required returns the 'base' major requirements"

Too nice of a claim; we can't really say anything about the meaning of the major requirements when a specialization is required, but none is provided...

Copy link
Member

@laggycomputer laggycomputer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

closer

{
ok: false,
message: "Couldn't find this major; check your ID?",
message: "Couldn't find this major associated with this specialization; check your ID?",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar is a bit funky here...

}),
specializationId: programIdBase.optional().openapi({
description:
"fetch major requirements when taking the specialization with this ID, if provided; providing no specialization when one is required results in unspecified behavior and is deprecated",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"fetch major requirements when taking the specialization with this ID, if provided; providing no specialization when one is required results in unspecified behavior and is deprecated",
"If provided, fetch major requirements given this specialization; providing no specialization when one is required has unspecified behavior",

`${schoolCode}-MAJOR-${majorCode}-${degreeCode}`,
majorAudit,
),
specCode: specCode,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use shorthand

Comment on lines +219 to +221
const inMap = this.parsedPrograms.get(
this.asMajorSpecId("Major in Chemistry"),
) as MajorProgram;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my goof as we discussed, but you can't as MajorProgram because this is an inappropriate assumption of infallibility in the map get.

Co-authored-by: Dante Dam <laggycomputer@yahoo.com>
Comment on lines 104 to +107
degree: string,
school: string,
majorCode: string,
specCode?: string,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More than one optional argument gets a little messy, especially if we add/reorder them. Do you want to pass in some kind of object instead? We could also leave this in if you don't care enough

// "optional american chemical society certification"
const inMap = this.parsedPrograms.get("Major in Chemistry") as MajorProgram;
return inMap ? [inMap[1]] : [];
const inMap = this.parsedPrograms.get(this.asMajorSpecId("Major in Chemistry"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the specialization argument to asMajorSpecId be nullable instead of optional, but that's an opinion.

majorId: varchar("major_id")
.notNull()
.references(() => major.id),
specId: varchar("spec_id").references(() => specialization.id),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should maybe be specialization_id for consistency

.primaryKey()
.generatedAlwaysAs(sql`jsonb_hash_extended(requirements, 0)`),
},
(table) => [],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this if we're not going to add any indices, etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

degreeworks scraper: silly typo in ON CONFLICTs Account for polymorphic major requirement blocks

2 participants